Skip to content

feat(01KVVJNM): a green story whose verdict step errors is surfaced as a re-runnable setup problem, never a false block#444

Merged
jackmcintyre merged 1 commit into
mainfrom
story/native-01kvvjnm-verdict-step-error-setup-problem
Jun 24, 2026
Merged

feat(01KVVJNM): a green story whose verdict step errors is surfaced as a re-runnable setup problem, never a false block#444
jackmcintyre merged 1 commit into
mainfrom
story/native-01kvvjnm-verdict-step-error-setup-problem

Conversation

@jackmcintyre

Copy link
Copy Markdown
Owner

What this changes (plain language)

When the run finishes a story whose PR is green and ACs are met, the last step asks for a final verdict. If that verdict step itself errors or hands back a result the run can't read, the old code lumped the story into the blocked pile as a generic verdict-failed — making an operator hand-rescue a story that was actually done and passing. This was a false failure.

This splits that catch-all so:

  • A genuine reviewer block (the reviewer really said BLOCKED) is still held back as a real quality decision — unchanged.
  • A verdict step that could not produce a result is now surfaced as a distinct, re-runnable setup problem (verdict-could-not-run), which names the verdict step so an operator can tell it apart from a review that could not run (review-could-not-run).

One concrete trigger: processReviewerTranscript throws ReviewerResultFileMalformedError when the reviewer-result file carries an unrecognised verdict token or isn't JSON; that surfaces as an MCP error with no next, so the run can't read a verdict — now classified as the setup problem instead of a false block.

The new reason is registered in the closed blocked_by enum so the manifest write doesn't throw and the reason is first-class (not silently swallowed). dist bundles rebuilt + committed.

Acceptance criteria

  • AC1 — A green, finished story whose verdict step errors/returns an unreadable result is surfaced as a re-runnable setup problem (verdict-could-not-run), never dumped as a verdict-failed the operator must rescue by hand.
  • AC2 — A genuine reviewer BLOCK is still held back as a real quality decision (reviewer-verdict-blocked), exactly as today; the new setup-problem handling applies ONLY when no real verdict could be produced.
  • AC3 — The setup-problem reason (verdict-could-not-run) names the verdict step distinctly from a review that could not run (review-could-not-run), so the operator can tell which part of the flow needs attention.

How it was tested

  • Extended run-fault-injection.test.ts to pin BOTH directions against the real run.workflow.js body: an injected unreadable verdict on a green story routes to verdict-could-not-run (asserted NOT verdict-failed, NOT review-could-not-run); a genuine done-blocked-reviewer-blocked still lands in reviewer-verdict-blocked.
  • Updated run-concurrent-dispatch.test.ts's reviewer-block fixture to inject the real done-blocked-reviewer-blocked verdict (it previously used a stand-in token that is now correctly reclassified as a verdict-step setup error).
  • pnpm -r build green; pnpm test:coverage green — 3828 tests pass, coverage lines 86.26% (≥86 CI gate).

https://claude.ai/code/session_01EE2ncqkqh3u5af93Uf3bDA

…re-runnable setup problem, not a false block

When the run's verdict step itself errors or returns an unreadable/unrecognised
result (e.g. processReviewerTranscript throws ReviewerResultFileMalformedError on
an unknown recommendedVerdict token or a non-JSON reviewer-result file -> MCP
isError with no `next`, or the courier relay garbles -> seam _parseError), the
old catch-all dumped the GREEN, finished story into the blocked pile as
'verdict-failed' for the operator to rescue by hand.

Split the verdict catch-all in run.workflow.js:
- done-blocked-reviewer-blocked  -> reviewer-verdict-blocked (unchanged quality block)
- everything else (no recognised verdict) -> verdict-could-not-run, a distinct,
  re-runnable SETUP problem whose reason names the VERDICT step so it reads
  distinctly from review-could-not-run (a review that could not RUN).

Registered verdict-could-not-run in the closed blocked_by enum
(execution-manifest.ts) so blockStory's manifest write does not throw and the
reason is first-class, not silently swallowed. Rebuilt + committed dist.

Tests: extended run-fault-injection to pin BOTH directions (unreadable verdict ->
verdict-could-not-run, never verdict-failed; genuine reviewer block ->
reviewer-verdict-blocked). Updated run-concurrent-dispatch's reviewer-block
fixture to inject the real done-blocked-reviewer-blocked verdict (was a stand-in
token now correctly reclassified).

Claude-Session: https://claude.ai/code/session_01EE2ncqkqh3u5af93Uf3bDA
@jackmcintyre jackmcintyre merged commit 1a780f0 into main Jun 24, 2026
1 check passed
@jackmcintyre jackmcintyre deleted the story/native-01kvvjnm-verdict-step-error-setup-problem branch June 24, 2026 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant